-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RAT-369: Add checkstyle and spotbugs to build and generate a report #238
Conversation
@Claudenw in case of any errors the build fails. I'm not sure if it's that helpful or if we remove this setting on the branch ..... |
In order to be able to bring changes into master I'd prefer to configure limits per module with the current threshold, thus we could make sure that no new stuff is being added and can fix the errors&warning in an incremental way - WDYT @Claudenw ? |
My thought was to turn on the spotbugs and fix any critical errors at that time (the ones with the red circles). |
@Claudenw the thing is that it either breaks the build or not. One can configure exclusions in an XML file in order to tolerate certain bugs explicitly ..... I'll have a look, but the first report said roughly up to 100 errors per submodule. |
Most of the errors relate to the fact that many file operations rely on the default charset. Unfortunately newer version of the JDK provide constructors to specify an encoding, but as RAT still relies on JDK8 these changes will be bigger. Example:
@Claudenw I changed the configuration to not fail the build temporarily, currently there are 68 errors left. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me. Just 2 real changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good. import * is the only real problem
apache-rat-plugin/src/main/java/org/apache/rat/mp/util/ignore/GlobIgnoreMatcher.java
Outdated
Show resolved
Hide resolved
apache-rat-core/src/main/java/org/apache/rat/config/parameters/ComponentType.java
Show resolved
Hide resolved
Could we merge this branch and fix the remaining errors step-by-step? Personally I'd prefer to have shorter-living branches ;) @Claudenw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. though I don't see any report of spotbugs being generated. I was hoping we could get something that would tell us if the number of bugs was decreasing over time.
…ty exclusion files
@Claudenw If you do not mind I would merge this one tomorrow :) Thanks for your feedback. |
Sounds good to me
…On Sun 5 May 2024, 21:50 P. Ottlinger, ***@***.***> wrote:
@Claudenw <https://github.com/Claudenw> If you do not mind I would merge
this one tomorrow :) Thanks for your feedback.
—
Reply to this email directly, view it on GitHub
<#238 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASTVHVXEWSFDQWGBUJP64TZA2EORAVCNFSM6AAAAABGKCHNKCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJUHEZDONBXGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@Claudenw I filed spotbugs/spotbugs-maven-plugin#804 to maybe get some feedback whether we can use spotbugs in a different way to allow failing the build if new errors are introduced. |
I verified locally (via ./.buildtools/generateStagingSiteInWebpageRepo) that a report is generated, but the build is not yet configured to fail.